Skip to content

feat: updated bulletins and new top chrome renderer#4705

Merged
cemreinanc merged 10 commits into
mainfrom
feat/updated-bulletins
May 12, 2026
Merged

feat: updated bulletins and new top chrome renderer#4705
cemreinanc merged 10 commits into
mainfrom
feat/updated-bulletins

Conversation

@cemreinanc
Copy link
Copy Markdown
Contributor

@cemreinanc cemreinanc commented May 9, 2026

Summary by CodeRabbit

  • New Features

    • Bulletins can be dismissed in-browser and (when signed in) synced to your account; bulletin visibility is more consistent.
    • Dynamic top chrome/header that adapts height for improved layout and sticky behavior.
    • Added "translated_by" i18n strings for several languages.
  • Bug Fixes

    • Improved HTML sanitization for bulletin content.
  • Chores

    • Refactored header/top-chrome architecture and simplified bulletin storage/display behavior server-side.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 9, 2026

📝 Walkthrough

Walkthrough

Replaces GlobalHeader with a composable TopChrome system; moves bulletins to server-side composition with sanitize-html and cookie-backed dismissal syncing; seeds route-aware header state on SSR; introduces measured top-chrome height CSS variable and hook; updates layouts, sticky offsets, APIs, and a Django migration removing bulletin post/project fields.

Changes

Bulletin & TopChrome Architecture

Layer / File(s) Summary
All changes (review entrypoint)
front_end/..., misc/..., front_end/package.json
Contains the coordinated migration: add sanitize-html, new bulletin/client/server flow, TopChrome composition and client measurement, top-chrome header context and server resolver, remove GlobalHeader, update layouts/pages, add migration removing bulletin.post/project, and sticky-offset + CSS var changes across many components.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • elisescu
  • ncarazon

Poem

🐰 I scrub the HTML and measure the top,
GlobalHeader hops away — then stop.
TopChrome sings, bulletins tidy and neat,
Cookies and server sync every discreet beat.
The header now fits — hip-hop, hop, hop!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/updated-bulletins

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (4)
front_end/package.json (1)

127-127: 💤 Low value

Minor type version mismatch with sanitize-html.

The @types/sanitize-html version is 2.16.1 while sanitize-html is 2.17.3. While this is typically compatible, consider updating the types package to match the library version more closely to ensure complete type coverage.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front_end/package.json` at line 127, Update the `@types` package to match the
sanitize-html library version: change the dependency entry for
"@types/sanitize-html" to a version that aligns with "sanitize-html" (e.g., bump
to 2.17.x) so type definitions better match the runtime; update the package.json
dependency for "@types/sanitize-html" and run the lockfile update (npm/yarn/pnpm
install) to ensure the new types are installed and consistent with the
"sanitize-html" package.
front_end/src/app/(services-quiz)/components/services_quiz_flow_content.tsx (1)

26-27: 💤 Low value

Consider safer type narrowing instead of assertion.

The type assertion step as ServicesQuizStepId bypasses type checking. While the fallback ?? STEP_COMPONENTS[1] handles invalid steps, consider whether step should already be properly typed from useServicesQuizFlow().

If step can legitimately be outside the valid range, the current approach is acceptable. Otherwise, fixing the upstream type would eliminate the need for the assertion.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front_end/src/app/`(services-quiz)/components/services_quiz_flow_content.tsx
around lines 26 - 27, The code is using a type assertion on step when picking
ActiveStep (ActiveStep, STEP_COMPONENTS, ServicesQuizStepId,
useServicesQuizFlow) which bypasses type safety; instead, ensure step is
properly typed upstream by updating useServicesQuizFlow to return
ServicesQuizStepId, or validate/narrow step before indexing (e.g., check it's an
integer and within STEP_COMPONENTS bounds or use a runtime existence check) and
then select STEP_COMPONENTS[step] with a safe fallback, removing the unsafe "as"
assertion.
front_end/src/app/(main)/components/top_chrome_client.tsx (1)

42-47: ⚡ Quick win

Consider adding error handling for MutationObserver.

While ResizeObserver.observe() is wrapped in try/catch (lines 60-66), the MutationObserver.observe() call is not. Although less common, MutationObserver.observe() can throw if the target node is invalid or options are malformed.

🛡️ Proposed error handling
      const observer = new MutationObserver(updateTopChromeHeight);
-     observer.observe(topChromeEl, {
-       attributes: true,
-       childList: true,
-       subtree: true,
-     });
+     try {
+       observer.observe(topChromeEl, {
+         attributes: true,
+         childList: true,
+         subtree: true,
+       });
+     } catch (error) {
+       logError(error, {
+         message: "Failed to observe top chrome height with MutationObserver",
+       });
+     }
      window.addEventListener("resize", updateTopChromeHeight);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front_end/src/app/`(main)/components/top_chrome_client.tsx around lines 42 -
47, Wrap the MutationObserver.observe call in a try/catch similar to the
ResizeObserver handling: when creating the observer (const observer = new
MutationObserver(updateTopChromeHeight)) surround observer.observe(topChromeEl,
{ attributes: true, childList: true, subtree: true }) with a try block and on
catch log the error (or handle it) using the same logger/cleanup pattern used
for ResizeObserver to avoid uncaught exceptions if topChromeEl is invalid or
options are malformed.
front_end/src/hooks/use_top_chrome_height.ts (1)

35-40: ⚡ Quick win

Consider defensive error handling for MutationObserver.

Similar to top_chrome_client.tsx, the observer.observe() call could benefit from try/catch protection, though failures are rare in practice.

🛡️ Optional defensive wrapper
     const observer = new MutationObserver(updateTopChromeHeight);
-    observer.observe(document.documentElement, {
-      attributes: true,
-      attributeFilter: ["style"],
-    });
+    try {
+      observer.observe(document.documentElement, {
+        attributes: true,
+        attributeFilter: ["style"],
+      });
+    } catch (error) {
+      // Log but don't fail; we've already set initial height
+      console.error("Failed to observe top chrome height:", error);
+    }
     window.addEventListener("resize", updateTopChromeHeight);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front_end/src/hooks/use_top_chrome_height.ts` around lines 35 - 40, Wrap the
MutationObserver.observe call in a try/catch to defensively handle rare
failures: when creating/starting the observer (the
`observer.observe(document.documentElement, { attributes: true, attributeFilter:
["style"] })` invocation in use_top_chrome_height.ts), catch any thrown error,
log or handle it (e.g., console.warn or a provided logger) and avoid crashing
the hook; still attach the `window.addEventListener("resize",
updateTopChromeHeight)` fallback so resize updates continue, and ensure you call
`observer.disconnect()` in the hook cleanup only if observation succeeded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@front_end/src/app/`(main)/components/bulletin.tsx:
- Around line 47-53: Replace the clickable FontAwesomeIcon with a real button
element and only render it when the dismissal handler exists: conditionally
render the control when onHidden is defined, use a <button> (instead of
FontAwesomeIcon alone) that contains the FontAwesomeIcon component, add
accessible attributes like aria-label and type="button" and ensure keyboard
activation calls onHidden; also replace any hardcoded English aria-label/text
with the translation string (e.g., via your existing translation
function/useTranslation) so the dismiss control is localized. Reference:
FontAwesomeIcon and onHidden in bulletin.tsx.

In `@front_end/src/app/`(main)/components/bulletins_client.tsx:
- Around line 48-55: The query function for React Query (queryFn) currently
catches errors from ClientMiscApi.getBulletins, logs them via logError, and
returns [] which overwrites valid SSR/initialData; change the catch to log the
error with logError but then rethrow (or throw the caught error) instead of
returning an empty array so React Query can preserve the last successful data
(leave ClientMiscApi.getBulletins and queryFn as the referenced symbols to
locate the code).
- Around line 76-79: The code adds bulletinId to
syncedDismissedBulletinIdsRef.current before the network call and never removes
it on failure, so transient errors permanently mark a dismiss as synced; change
the logic in the dismiss flow that uses syncedDismissedBulletinIdsRef and
dismissBulletin so the id is only added after dismissBulletin resolves
successfully, or if you must optimistically add it then remove it inside the
catch handler (using syncedDismissedBulletinIdsRef.current.delete(bulletinId))
and rethrow/log via logError to allow retries; ensure all references to
syncedDismissedBulletinIdsRef, dismissBulletin, logError, and bulletinId are
updated accordingly.
- Around line 37-42: The component seeds dismissedBulletinIds and
syncedDismissedBulletinIdsRef only on initial mount so client state doesn't
update when new initialDismissedBulletinIds/initialSyncedDismissedBulletinIds
props arrive (causing dismissal leakage across sessions); add a useEffect in the
same component (bulletins_client.tsx) that watches initialDismissedBulletinIds
and initialSyncedDismissedBulletinIds and calls setDismissedBulletinIds(() =>
new Set(initialDismissedBulletinIds)) and sets
syncedDismissedBulletinIdsRef.current = new
Set(initialSyncedDismissedBulletinIds) to reset the state/ref whenever the prop
values change.

In `@front_end/src/app/`(main)/components/bulletins_shared.ts:
- Around line 13-19: The current parsing chain (value.split(...).map(id =>
Number.parseInt(id, 10)).filter(Number.isFinite)) accepts malformed entries like
"12abc", negatives and decimals; replace it with strict positive-integer
validation: when mapping each raw id from value.split(...) validate the raw
string with a digits-only check (e.g. /^\d+$/) or use Number and
Number.isInteger plus > 0 before including it, and then dedupe as before; update
both occurrences (the shown parseInt usage and the same block at lines 23-27) to
only accept strictly positive integers and reject all other inputs.

In `@front_end/src/app/`(main)/components/content_translated_banner/index.tsx:
- Around line 56-58: The span contains a hardcoded English label "translated
by"; replace it with the i18n helper by importing and using useTranslations()
and calling t(...) where this component (e.g., the ContentTranslatedBanner
component or its functional scope) renders that span; update the JSX to use
t("translated_by") (or the appropriate key you add to your translation files) so
the string is localized and ensure the translation key exists in the locale JSON
used by useTranslations().

In `@front_end/src/app/`(main)/notebooks/components/notebook_content_sections.tsx:
- Around line 84-87: The TOC active-section offset ignores dynamic chrome height
on desktop: update the offset calculation used when computing activeId so it
includes topChromeHeight for both branches (i.e., change the isLargeScreen
branch from using DESKTOP_SCROLL_OFFSET alone to DESKTOP_SCROLL_OFFSET +
topChromeHeight or apply a bulletin delta before computing activeId). Locate the
variables and logic around offset, isLargeScreen, DESKTOP_SCROLL_OFFSET,
topChromeHeight and activeId in notebook_content_sections.tsx and ensure the
desktop path uses the real chrome height when computing the scroll offset for
active section highlighting.

In
`@front_end/src/app/`(prediction-flow)/tournament/[slug]/prediction-flow/loading.tsx:
- Around line 29-41: The loading header shows exit controls that are
non-functional; update the two Button usages inside FlowHeaderActions (the text
button that renders t("exitPredictionFlow") and the icon-only Button with
FontAwesomeIcon/faRightFromBracket) to call the real exit handler (pass the
existing exit function or navigation callback as an onClick or provide an href)
and ensure the icon-only Button has an accessible label (e.g., aria-label or
aria-labelledby) so screen readers can announce the action; keep
styling/variants the same while wiring these Buttons to the actual exit
behavior.

In `@misc/models.py`:
- Around line 47-48: The Bulletin model's __str__ currently slices raw HTML from
self.text which can leak tags; update Bulletin.__str__ to first convert/sanitize
the stored HTML to plain text (e.g., use django.utils.html.strip_tags or an
equivalent HTML-to-text helper) then truncate that plaintext to 150 characters
and append "..." when longer; add the needed import (strip_tags) and ensure you
call the plain-text variable in the return value instead of self.text.

In `@misc/views.py`:
- Around line 143-145: The code directly calls Bulletin.objects.get(pk=pk)
inside BulletinViewedBy.objects.get_or_create which raises Bulletin.DoesNotExist
and causes a 500; first resolve the Bulletin using a 404-safe lookup (e.g.,
Django's get_object_or_404 or catch Bulletin.DoesNotExist and raise Http404)
into a local variable (e.g., bulletin) and then call
BulletinViewedBy.objects.get_or_create(bulletin=bulletin, user=user) so
stale/invalid ids return a 404 instead of a server error.

---

Nitpick comments:
In `@front_end/package.json`:
- Line 127: Update the `@types` package to match the sanitize-html library
version: change the dependency entry for "@types/sanitize-html" to a version
that aligns with "sanitize-html" (e.g., bump to 2.17.x) so type definitions
better match the runtime; update the package.json dependency for
"@types/sanitize-html" and run the lockfile update (npm/yarn/pnpm install) to
ensure the new types are installed and consistent with the "sanitize-html"
package.

In `@front_end/src/app/`(main)/components/top_chrome_client.tsx:
- Around line 42-47: Wrap the MutationObserver.observe call in a try/catch
similar to the ResizeObserver handling: when creating the observer (const
observer = new MutationObserver(updateTopChromeHeight)) surround
observer.observe(topChromeEl, { attributes: true, childList: true, subtree: true
}) with a try block and on catch log the error (or handle it) using the same
logger/cleanup pattern used for ResizeObserver to avoid uncaught exceptions if
topChromeEl is invalid or options are malformed.

In `@front_end/src/app/`(services-quiz)/components/services_quiz_flow_content.tsx:
- Around line 26-27: The code is using a type assertion on step when picking
ActiveStep (ActiveStep, STEP_COMPONENTS, ServicesQuizStepId,
useServicesQuizFlow) which bypasses type safety; instead, ensure step is
properly typed upstream by updating useServicesQuizFlow to return
ServicesQuizStepId, or validate/narrow step before indexing (e.g., check it's an
integer and within STEP_COMPONENTS bounds or use a runtime existence check) and
then select STEP_COMPONENTS[step] with a safe fallback, removing the unsafe "as"
assertion.

In `@front_end/src/hooks/use_top_chrome_height.ts`:
- Around line 35-40: Wrap the MutationObserver.observe call in a try/catch to
defensively handle rare failures: when creating/starting the observer (the
`observer.observe(document.documentElement, { attributes: true, attributeFilter:
["style"] })` invocation in use_top_chrome_height.ts), catch any thrown error,
log or handle it (e.g., console.warn or a provided logger) and avoid crashing
the hook; still attach the `window.addEventListener("resize",
updateTopChromeHeight)` fallback so resize updates continue, and ensure you call
`observer.disconnect()` in the hook cleanup only if observation succeeded.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0bff60ed-86d9-430e-aec2-a2a4adfd2389

📥 Commits

Reviewing files that changed from the base of the PR and between 92f81d6 and efa912a.

⛔ Files ignored due to path filters (1)
  • front_end/bun.lock is excluded by !**/*.lock
📒 Files selected for processing (77)
  • front_end/package.json
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/contest-rules/page.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/how-it-works/page.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/layout.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/leaderboards/page.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/notice-at-collection/page.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/page.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/q1/page.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-reg/layout.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-reg/page.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater/contest-rules/page.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater/how-it-works/page.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater/layout.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater/leaderboards/page.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater/notice-at-collection/page.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater/page.tsx
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater/q1/page.tsx
  • front_end/src/app/(campaigns-registration)/id-verification/page.tsx
  • front_end/src/app/(campaigns-registration)/rand/confirm/page.tsx
  • front_end/src/app/(campaigns-registration)/rand/contest-rules/page.tsx
  • front_end/src/app/(campaigns-registration)/rand/layout.tsx
  • front_end/src/app/(campaigns-registration)/rand/page.tsx
  • front_end/src/app/(futureeval)/futureeval/components/futureeval-navbar.tsx
  • front_end/src/app/(main)/(tournaments)/tournaments/components/tournaments_header.tsx
  • front_end/src/app/(main)/accounts/actions.ts
  • front_end/src/app/(main)/actions.ts
  • front_end/src/app/(main)/aggregation-explorer/components/aggregation_graph_panel.tsx
  • front_end/src/app/(main)/c/[slug]/page.tsx
  • front_end/src/app/(main)/c/[slug]/settings/page.tsx
  • front_end/src/app/(main)/components/bulletin.tsx
  • front_end/src/app/(main)/components/bulletins.tsx
  • front_end/src/app/(main)/components/bulletins_client.tsx
  • front_end/src/app/(main)/components/bulletins_shared.ts
  • front_end/src/app/(main)/components/content_translated_banner/index.tsx
  • front_end/src/app/(main)/components/headers/community_header.tsx
  • front_end/src/app/(main)/components/headers/global_header.tsx
  • front_end/src/app/(main)/components/headers/header.tsx
  • front_end/src/app/(main)/components/impersonation_banner_client.tsx
  • front_end/src/app/(main)/components/impersonation_banner_server.tsx
  • front_end/src/app/(main)/components/top_chrome.tsx
  • front_end/src/app/(main)/components/top_chrome_client.tsx
  • front_end/src/app/(main)/components/top_chrome_error_boundary.tsx
  • front_end/src/app/(main)/components/top_chrome_header_context.tsx
  • front_end/src/app/(main)/components/top_chrome_header_server.ts
  • front_end/src/app/(main)/components/top_chrome_header_shared.ts
  • front_end/src/app/(main)/components/top_chrome_header_slot.tsx
  • front_end/src/app/(main)/error.tsx
  • front_end/src/app/(main)/labor-hub/components/labor_hub_navigation.tsx
  • front_end/src/app/(main)/layout.tsx
  • front_end/src/app/(main)/not-found.tsx
  • front_end/src/app/(main)/notebooks/[id]/[[...slug]]/page_compotent.tsx
  • front_end/src/app/(main)/notebooks/components/notebook_content_sections.tsx
  • front_end/src/app/(main)/questions/[id]/[[...slug]]/page_component.tsx
  • front_end/src/app/(main)/questions/components/sidebar.tsx
  • front_end/src/app/(main)/questions/create/[content_type]/page.tsx
  • front_end/src/app/(main)/questions/create/page.tsx
  • front_end/src/app/(main)/questions/page.tsx
  • front_end/src/app/(prediction-flow)/components/header.tsx
  • front_end/src/app/(prediction-flow)/tournament/[slug]/prediction-flow/loading.tsx
  • front_end/src/app/(prediction-flow)/tournament/[slug]/prediction-flow/page.tsx
  • front_end/src/app/(services-quiz)/components/services_quiz_flow_content.tsx
  • front_end/src/app/(services-quiz)/components/services_quiz_header.tsx
  • front_end/src/app/(services-quiz)/components/services_quiz_screen.tsx
  • front_end/src/app/(storefront)/layout.tsx
  • front_end/src/app/not-found.tsx
  • front_end/src/components/flow/flow_header.tsx
  • front_end/src/components/markdown_editor/editor.css
  • front_end/src/components/ui/tabs/index.tsx
  • front_end/src/hooks/use_top_chrome_height.ts
  • front_end/src/services/api/misc/misc.shared.ts
  • front_end/src/utils/navigation.ts
  • front_end/tailwind.config.ts
  • misc/admin.py
  • misc/migrations/0009_remove_bulletin_post_project.py
  • misc/models.py
  • misc/urls.py
  • misc/views.py
💤 Files with no reviewable changes (5)
  • front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-reg/page.tsx
  • front_end/src/app/(main)/components/headers/global_header.tsx
  • front_end/src/utils/navigation.ts
  • front_end/src/app/(campaigns-registration)/rand/page.tsx
  • front_end/src/app/(campaigns-registration)/rand/confirm/page.tsx

Comment thread front_end/src/app/(main)/components/bulletin.tsx Outdated
Comment thread front_end/src/app/(main)/components/bulletins_client.tsx
Comment thread front_end/src/app/(main)/components/bulletins_client.tsx
Comment thread front_end/src/app/(main)/components/bulletins_client.tsx Outdated
Comment thread front_end/src/app/(main)/components/bulletins_shared.ts
Comment thread front_end/src/app/(prediction-flow)/tournament/[slug]/prediction-flow/loading.tsx Outdated
Comment thread misc/models.py Outdated
Comment thread misc/views.py Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 9, 2026

Cleanup: Preview Environment Removed

The preview environment for this PR has been destroyed.

Resource Status
🌐 Preview App Deleted
🗄️ PostgreSQL Branch Deleted
⚡ Redis Database Deleted
🔧 GitHub Deployments Removed
📦 Docker Image Retained (auto-cleanup via GHCR policies)

Cleanup triggered by PR close at 2026-05-12T14:21:48Z

@cemreinanc cemreinanc marked this pull request as draft May 9, 2026 21:47
@cemreinanc cemreinanc marked this pull request as ready for review May 11, 2026 12:20
Comment thread misc/views.py Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
front_end/src/app/(main)/components/bulletin.tsx (1)

59-65: 💤 Low value

Confirm suppressHydrationWarning is necessary here.

The sanitized output is derived deterministically from text, so server/client output should match. Using suppressHydrationWarning here risks masking real mismatches (e.g., if upstream cookie/locale-dependent text leaks into bulletin content). If you don't have a concrete hydration mismatch to silence, consider removing it.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front_end/src/app/`(main)/components/bulletin.tsx around lines 59 - 65, The
div rendering sanitizedText is using suppressHydrationWarning unnecessarily;
remove the suppressHydrationWarning prop from the div in bulletin.tsx (the
element that sets dangerouslySetInnerHTML with __html: sanitizedText) unless you
have a confirmed hydration mismatch. Instead, ensure sanitizedText is computed
deterministically from the incoming text prop (no client-only sources like
cookies/locale) and run the app to confirm there are no hydration warnings; if
you must keep it, add a short comment explaining the concrete mismatch and why
suppressHydrationWarning is required.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@front_end/src/app/`(main)/components/bulletin.tsx:
- Around line 17-31: The transformTags handler for "a" should always ensure rel
includes both "noopener" and "noreferrer" when attribs.target === "_blank":
update the logic in the transformTags.a function to read the existing
attribs.rel (if any), split it into tokens, add "noopener" and "noreferrer" to
the set, then rejoin and assign attribs.rel to the merged value before
returning; keep returning the original tagName and merged attribs otherwise. Use
the existing symbols transformTags and the a tag handler and operate on
attribs.target and attribs.rel to locate and fix the code.

---

Nitpick comments:
In `@front_end/src/app/`(main)/components/bulletin.tsx:
- Around line 59-65: The div rendering sanitizedText is using
suppressHydrationWarning unnecessarily; remove the suppressHydrationWarning prop
from the div in bulletin.tsx (the element that sets dangerouslySetInnerHTML with
__html: sanitizedText) unless you have a confirmed hydration mismatch. Instead,
ensure sanitizedText is computed deterministically from the incoming text prop
(no client-only sources like cookies/locale) and run the app to confirm there
are no hydration warnings; if you must keep it, add a short comment explaining
the concrete mismatch and why suppressHydrationWarning is required.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 75fca017-f26a-4158-8bba-adc40848e0ab

📥 Commits

Reviewing files that changed from the base of the PR and between efa912a and c3d350b.

⛔ Files ignored due to path filters (1)
  • front_end/bun.lock is excluded by !**/*.lock
📒 Files selected for processing (19)
  • front_end/messages/cs.json
  • front_end/messages/en.json
  • front_end/messages/es.json
  • front_end/messages/pt.json
  • front_end/messages/zh-TW.json
  • front_end/messages/zh.json
  • front_end/package.json
  • front_end/src/app/(main)/components/bulletin.tsx
  • front_end/src/app/(main)/components/bulletins_client.tsx
  • front_end/src/app/(main)/components/bulletins_shared.ts
  • front_end/src/app/(main)/components/content_translated_banner/index.tsx
  • front_end/src/app/(main)/components/top_chrome_client.tsx
  • front_end/src/app/(main)/notebooks/components/notebook_content_sections.tsx
  • front_end/src/app/(prediction-flow)/tournament/[slug]/prediction-flow/loading.tsx
  • front_end/src/app/(prediction-flow)/tournament/[slug]/prediction-flow/loading_header_actions.tsx
  • front_end/src/app/(services-quiz)/components/services_quiz_flow_content.tsx
  • front_end/src/hooks/use_top_chrome_height.ts
  • misc/models.py
  • misc/views.py
✅ Files skipped from review due to trivial changes (6)
  • front_end/messages/zh-TW.json
  • front_end/messages/zh.json
  • front_end/messages/pt.json
  • front_end/messages/en.json
  • front_end/messages/es.json
  • front_end/messages/cs.json
🚧 Files skipped from review as they are similar to previous changes (10)
  • front_end/package.json
  • front_end/src/hooks/use_top_chrome_height.ts
  • front_end/src/app/(main)/notebooks/components/notebook_content_sections.tsx
  • front_end/src/app/(main)/components/content_translated_banner/index.tsx
  • front_end/src/app/(main)/components/bulletins_shared.ts
  • front_end/src/app/(main)/components/top_chrome_client.tsx
  • misc/models.py
  • front_end/src/app/(services-quiz)/components/services_quiz_flow_content.tsx
  • front_end/src/app/(main)/components/bulletins_client.tsx
  • misc/views.py

Comment on lines +17 to +31
transformTags: {
a: (tagName, attribs) => {
if (attribs.target === "_blank" && !attribs.rel) {
return {
tagName,
attribs: {
...attribs,
rel: "noopener noreferrer",
},
};
}

return { tagName, attribs };
},
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Always force rel="noopener noreferrer" for target="_blank" links.

The current rule only injects rel when it is absent. Admin-authored bulletin HTML containing <a target="_blank" rel="opener"> (or any other non-empty rel value missing noopener/noreferrer) will pass through unchanged, leaving the tabnabbing/window.opener vector intact. Always merge the safe tokens regardless of existing rel.

🛡️ Proposed fix
     transformTags: {
       a: (tagName, attribs) => {
-        if (attribs.target === "_blank" && !attribs.rel) {
-          return {
-            tagName,
-            attribs: {
-              ...attribs,
-              rel: "noopener noreferrer",
-            },
-          };
-        }
-
-        return { tagName, attribs };
+        if (attribs.target !== "_blank") {
+          return { tagName, attribs };
+        }
+        const existing = (attribs.rel ?? "").split(/\s+/).filter(Boolean);
+        for (const token of ["noopener", "noreferrer"]) {
+          if (!existing.includes(token)) existing.push(token);
+        }
+        return {
+          tagName,
+          attribs: { ...attribs, rel: existing.join(" ") },
+        };
       },
     },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
transformTags: {
a: (tagName, attribs) => {
if (attribs.target === "_blank" && !attribs.rel) {
return {
tagName,
attribs: {
...attribs,
rel: "noopener noreferrer",
},
};
}
return { tagName, attribs };
},
},
transformTags: {
a: (tagName, attribs) => {
if (attribs.target !== "_blank") {
return { tagName, attribs };
}
const existing = (attribs.rel ?? "").split(/\s+/).filter(Boolean);
for (const token of ["noopener", "noreferrer"]) {
if (!existing.includes(token)) existing.push(token);
}
return {
tagName,
attribs: { ...attribs, rel: existing.join(" ") },
};
},
},
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front_end/src/app/`(main)/components/bulletin.tsx around lines 17 - 31, The
transformTags handler for "a" should always ensure rel includes both "noopener"
and "noreferrer" when attribs.target === "_blank": update the logic in the
transformTags.a function to read the existing attribs.rel (if any), split it
into tokens, add "noopener" and "noreferrer" to the set, then rejoin and assign
attribs.rel to the merged value before returning; keep returning the original
tagName and merged attribs otherwise. Use the existing symbols transformTags and
the a tag handler and operate on attribs.target and attribs.rel to locate and
fix the code.

Copy link
Copy Markdown
Contributor

@ncarazon ncarazon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cemreinanc cemreinanc merged commit 32e478f into main May 12, 2026
18 checks passed
@cemreinanc cemreinanc deleted the feat/updated-bulletins branch May 12, 2026 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants